cli: show debug options on --verbose #9693
Conversation
|
They are documented in wiki, https://github.com/iterative/dvc/wiki/Debugging,-Profiling-and-Benchmarking-DVC, although they may be hard to find.
It only works on macOS and Linux. On macOS, you have to press |
|
Thanks for the pointer! It probably makes sense to replace the blurb I wrote in the contributing docs with a reference to this wiki. I think having this info in contributing docs will make it a lot easier to find because the people who would need debugging tools (i.e. contributors) should be reading that. |
|
I updated the help strings based on info in the wiki, and I also modified the docs PR to point to the wiki. New |
dvc/_debug.py
Outdated
| parser.add_argument("--yappi", action="store_true", default=False, help=SUPPRESS) | ||
| # For detailed info see: | ||
| # https://github.com/iterative/dvc/wiki/Debugging,-Profiling-and-Benchmarking-DVC | ||
| dvc_show_debug_options = os.environ.get("DVC_SHOW_DEBUG_OPTIONS", "") |
There was a problem hiding this comment.
Seems a bit too verbose. Maybe better to use --debug flag? Do you know maybe some examples of how other projects handle this?
There was a problem hiding this comment.
In the long term, I'd really like to introduce the concept of "extensions", even if it's for our own use. We could enable extra functionalities if the devel extension is enabled, for example, in the config.
There was a problem hiding this comment.
I tend to opt for verbosity when I'm unsure. I think DVC_DEBUG=1 <cmd> would likely work as well.
I don't see an easy way to add a --debug option because the argparse object hasn't finished being constructed at the time where we need to determine if we use SUPRESS or add a help message. I think the environment variable approach is the cleanest way to allow these options to be conditionally exposed in --help.
The other options I can think of are:
-
Add a
--debugflag which prints a special help section, but that would need to be maintained separately, so that's undesirable. -
Check if
"--debug" in sys.argv, but that's undesirable because 1. the parser might not be applied to sys.argv, and 2. there is a chance--debugmight appear on the command line, but not be intended as an argument flag (e.g.dvc add -- --debug). Checkingsys.argvwon't respect the--like argparse will. These issues probably won't arise in practice, and even if they did it would likely not matter because it just changes the content of the help message. Still, I think it is unclean design and the environment variable approach seems like the "idomatic" thing to do here. That being said,"--debug" in sys.argvis simple, and I'm open to doing it this way. The only thing I think is important is that there is a way for a developer to unhide these options.
There was a problem hiding this comment.
@skshetry Seems like a total overkill for this purpose. But I'm not even sure what extensions you mean, I suppose like git extensions? Don't quite see the need.
There was a problem hiding this comment.
@Erotemic DVC_DEBUG sounds fine to me, let's not overthink it.
There was a problem hiding this comment.
Looking at mercurial, they show the options only when --verbose is set including debugging related options.
There was a problem hiding this comment.
@Erotemic regarding the cli option, there is a way to partially parse known options in argparse IIRC. Just checking argv (not sys.argv) is also fine. Mercurial's --verbose sounds pretty good too.
There was a problem hiding this comment.
Switched to DVC_DEBUG and added it to env.
I do like the idea of showing the extra help with vebose is set. At first I wasn't sure how to do it, but I think its possible as long as the parser is initialized with add_help=False (which it is) and if we can ensure the verbose argument is added to the parser before we call add_debugging_flags, which should just require reordering the code in get_parent_parser.
Now that I see how to do this, I think it would be a good idea to remove the DVC_DEBUG environment variable entirely, and just add the extra help options is -v is already given (which will make it much easier for a developer to stumble on this too, which is good).
There was a problem hiding this comment.
I made the above change, I can revert if you want to go with the environ, but I think just respecting -v is much cleaner now that I know about the add_help argment to ArgumentParser.
|
While playing with this I noticed two things and made changes accordingly. First the "cprofile-dump" option was not near "cprofile", so I moved it to keep similar options grouped. Secondly, if you run |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #9693 +/- ##
==========================================
+ Coverage 90.53% 90.55% +0.02%
==========================================
Files 480 480
Lines 36382 36392 +10
Branches 5101 5228 +127
==========================================
+ Hits 32938 32955 +17
+ Misses 2852 2849 -3
+ Partials 592 588 -4
☔ View full report in Codecov by Sentry. |
[pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci
line length linelen lint
Co-authored-by: skshetry <[email protected]>
skshetry
changed the title
Digging through the code I saw there were a lot of builtin tools for profiling / debugging, but (as far as I can tell) there was no mention of these in the docs, nor was there wany way for a user to become aware of them without digging into the code.
To remedy this, I added an environment variable
DVC_SHOW_DEBUG_OPTIONS(which perhaps could be shorted toDVC_DEBUG), and if this is specified the hidden CLI options will be shown to the user. Otherwise the behavior is the same.Also, when I tried to run a command with
--show-stackI didn't see any difference in the output. So I'm not sure what it's actually doing.I've added a section in the contributing docs which mentions this new option. The corresponding PR is here: iterative/dvc.org#4662